Skip to content

Conversation

@melissalinkert
Copy link
Member

Fixes #288.

This lists valid space and time units per https://ngff.openmicroscopy.org/latest/#axes-md, and rejects anything not included in those lists. As indicated in #144, pixel is one of several units that are supported by OME-XML schema but not NGFF.

I debated whether to implement this by defining NGFF schema units, or defining just units that don't overlap between the two schemas (as in #144 (comment)). I think the former approach is going to be easier to keep up to date, but happy to hear other opinions.

With the test data mentioned in #288, bioformats2raw --debug mri.ome.tiff mri.zarr should log that pixel units are being omitted, and mri.zarr/0/.zattrs should show no unit attribute for anything under axes.

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using bioformats2raw 0.10.1, I was able to reproduce the original issue either using the Fiji workflow described in #288 (Open the MRI sample and export as OME-TIFF) or a synthetic sample file like test&physicalSizeX=1pixel&physicalSizeY=1pixel.fake. In both cases, the conversion generates a Zarr dataset where the unit of the x and y the physical axes are expressed in pixel.

With a build of bioformats2raw including this PR, the Zarr datasets generated by the conversion are binary identical except for the physical axes which are now unitless

sbesson@Sebastien-GS-MacBook-Pro-2025 bioformats2raw % diff -ur test.zarr test_pr289.zarr 
diff -ur test.zarr/0/.zattrs test_pr289.zarr/0/.zattrs
--- test.zarr/0/.zattrs	2025-09-04 08:46:45
+++ test_pr289.zarr/0/.zattrs	2025-09-04 08:54:26
@@ -14,11 +14,9 @@
       "name" : "z",
       "type" : "space"
     }, {
-      "unit" : "pixel",
       "name" : "y",
       "type" : "space"
     }, {
-      "unit" : "pixel",
       "name" : "x",
       "type" : "space"
     } ],
sbesson@Sebastien-GS-MacBook-Pro-2025 bioformats2raw % diff -ur mri.zarr mri_pr289.zarr   
diff -ur mri.zarr/0/.zattrs mri_pr289.zarr/0/.zattrs
--- mri.zarr/0/.zattrs	2025-09-04 08:45:28
+++ mri_pr289.zarr/0/.zattrs	2025-09-04 08:54:19
@@ -11,15 +11,12 @@
       "name" : "c",
       "type" : "channel"
     }, {
-      "unit" : "pixel",
       "name" : "z",
       "type" : "space"
     }, {
-      "unit" : "pixel",
       "name" : "y",
       "type" : "space"
     }, {
-      "unit" : "pixel",
       "name" : "x",
       "type" : "space"
     } ],

The proposed changes are consistent with the discussion in #144 and feels like a reasonable approach given the behavior of other tools in the NGFF ecosystem. /cc @ome/ngff

Could we add a unit test using fake files to capture the expected behavior?

@melissalinkert
Copy link
Member Author

The proposed changes are consistent with the discussion in #144 and feels like a reasonable approach given the behavior of other tools in the NGFF ecosystem. /cc @ome/ngff

Looks like the team mention might not have worked (maybe due to different organization?) - cc @joshmoore.

Could we add a unit test using fake files to capture the expected behavior?

Done in 4b39f7e.

@sbesson sbesson self-requested a review September 9, 2025 15:15
@sbesson sbesson merged commit 2e7c1ef into glencoesoftware:master Sep 9, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit "pixel" propagated to NGFF axes causes Neuroglancer parse error

2 participants